Skip to content

Add analyzer to suggest input parameter binding over GetInput() #550

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
YunchuWang merged 7 commits into from
Dec 18, 2025
Merged

Add analyzer to suggest input parameter binding over GetInput() #550

YunchuWang merged 7 commits into from
Dec 18, 2025

Conversation

Copy link
Contributor

Copilot AI commented Dec 10, 2025 •
edited
Loading

Summary

What changed?

  • Added new Roslyn analyzer GetInputOrchestrationAnalyzer (DURABLE0009) that detects usage of ctx.GetInput<T>() in orchestration methods
  • Analyzer suggests using input parameter binding instead, which is the preferred pattern
  • Added comprehensive test coverage with 10 test cases covering all orchestration patterns
  • Registered the new diagnostic ID in AnalyzerReleases.Unshipped.md

Why is this change needed?

The GetInput<T>() method is marked as potentially deprecated and input parameter binding is the recommended approach for accessing orchestration input. This Info-level analyzer helps guide developers toward the preferred pattern without blocking their builds.

Issues / work items

Project checklist

  • Release notes are not required for the next release
  • Backport is not required
  • All required tests have been added/updated (unit tests, E2E tests)
  • Breaking change? No

AI-assisted code disclosure (required)

Was an AI tool used?

  • Yes, an AI agent generated most of this PR

If AI was used:

  • Tool(s): GitHub Copilot Workspace AI Agent
  • AI-assisted areas/files:
    • src/Analyzers/Orchestration/GetInputOrchestrationAnalyzer.cs (analyzer implementation)
    • test/Analyzers.Tests/Orchestration/GetInputOrchestrationAnalyzerTests.cs (test suite)
    • src/Analyzers/Resources.resx (diagnostic messages)
    • src/Analyzers/AnalyzerReleases.Unshipped.md (release tracking)
  • What you changed after AI output:
    • Added null check for operation.TargetMethod based on code review feedback
    • Changed test verifier from CSharpCodeFixVerifier to CSharpAnalyzerVerifier for consistency
    • Resolved merge conflict with main branch (removed DURABLE2003/2004 entries that were shipped)

AI verification:

  • I understand the code and can explain it
  • I verified referenced APIs/types exist and are correct
  • I reviewed edge cases/failure paths (timeouts, retries, cancellation, exceptions)
  • I reviewed concurrency/async behavior
  • I checked for unintended breaking or behavior changes

Testing

Automated tests

  • Result: Passed
  • All 119 analyzer tests pass (including 10 new tests for GetInputOrchestrationAnalyzer)
  • Tests cover all orchestration patterns: Azure Functions, TaskOrchestrator, OrchestratorFunc, nested calls, and multiple invocations

Manual validation

  • Environment: Visual Studio Code with C# Dev Kit
  • Steps + observed results:
    1. Opened sample with context.GetInput<T>() usage
    2. Observed Info-level diagnostic (DURABLE0009) with message: "Consider using an input parameter instead of 'GetInput()' in orchestration 'HelloSequenceOrchestrator'"
    3. Verified diagnostic appears for all orchestration patterns
  • Evidence: Screenshot provided by @YunchuWang showing analyzer working in IDE

Notes for reviewers

  • This is an Info-level diagnostic (not a warning or error), so it won't block builds
  • The analyzer works across all orchestration patterns: Azure Functions with [OrchestrationTrigger], TaskOrchestrator<TInput, TOutput> classes, and AddOrchestratorFunc lambdas
  • Follows the same pattern as other orchestration analyzers in the codebase
  • No code fixer is provided (just a suggestion), so tests use CSharpAnalyzerVerifier instead of CSharpCodeFixVerifier
Original prompt

This section details on the original issue you should resolve

<issue_title>Roslyn Analyzer could suggest input parameter binding instead of ctx.GetInput()</issue_title>
<issue_description>We could have an Info level diagnostic informing users that an input parameter binding can be used instead of ctx.GetInput<T>()</issue_description>

Comments on the Issue (you are @copilot in this section)

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Add diagnostic for input parameter binding suggestion Add analyzer to suggest input parameter binding over GetInput<T>() Dec 10, 2025
Copilot AI requested a review from YunchuWang December 10, 2025 19:08
@YunchuWang
Copy link
Member

YunchuWang commented Dec 18, 2025

@copilot resolve conflict

Copy link
Contributor Author

Copilot AI commented Dec 18, 2025

@copilot resolve conflict

Conflict resolved in commit e5ccff1. The issue was that DURABLE2003 and DURABLE2004 were shipped to main branch, so I removed them from AnalyzerReleases.Unshipped.md, keeping only DURABLE0009. All 119 analyzer tests pass after the merge.

@YunchuWang YunchuWang marked this pull request as ready for review December 18, 2025 17:43
Copilot AI review requested due to automatic review settings December 18, 2025 17:43
Copilot AI reviewed Dec 18, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements a new Roslyn analyzer (DURABLE0009) that suggests using input parameter binding instead of calling ctx.GetInput<T>() in orchestration methods. This is an informational diagnostic to guide developers toward the preferred pattern, especially since GetInput<T>() is marked as potentially deprecated in the codebase.

Key Changes:

  • Implements GetInputOrchestrationAnalyzer with Info severity to detect ctx.GetInput<T>() usage
  • Adds comprehensive test coverage for various orchestration patterns (Durable Functions, TaskOrchestrator, and OrchestratorFunc)
  • Includes resource strings for diagnostic messages

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/Analyzers/Orchestration/GetInputOrchestrationAnalyzer.cs New analyzer detecting GetInput() usage in orchestration methods and suggesting input parameter binding
test/Analyzers.Tests/Orchestration/GetInputOrchestrationAnalyzerTests.cs Comprehensive test suite covering empty code, non-orchestrations, various orchestration patterns, nested calls, and multiple invocations
src/Analyzers/Resources.resx Resource strings for analyzer title and message format
src/Analyzers/AnalyzerReleases.Unshipped.md Documentation entry for new DURABLE0009 diagnostic

@YunchuWang
Copy link
Member

YunchuWang commented Dec 18, 2025

image verified working

@YunchuWang
Copy link
Member

YunchuWang commented Dec 18, 2025

@copilot update pr description to follow .github/PULL_REQUEST_TEMPLATE.md

@YunchuWang
Copy link
Member

YunchuWang commented Dec 18, 2025

LGTM

Copy link
Contributor Author

Copilot AI commented Dec 18, 2025

@copilot update pr description to follow .github/PULL_REQUEST_TEMPLATE.md

Updated PR description to follow .github/PULL_REQUEST_TEMPLATE.md:

Summary

What changed?

  • Added new Roslyn analyzer GetInputOrchestrationAnalyzer (DURABLE0009) that detects usage of ctx.GetInput<T>() in orchestration methods
  • Analyzer suggests using input parameter binding instead, which is the preferred pattern
  • Added comprehensive test coverage with 10 test cases covering all orchestration patterns
  • Registered the new diagnostic ID in AnalyzerReleases.Unshipped.md

Why is this change needed?

The GetInput<T>() method is marked as potentially deprecated and input parameter binding is the recommended approach for accessing orchestration input. This Info-level analyzer helps guide developers toward the preferred pattern without blocking their builds.

Issues / work items

Project checklist

  • Release notes are not required for the next release
  • Backport is not required
  • All required tests have been added/updated (unit tests, E2E tests)
  • Breaking change? No

AI-assisted code disclosure (required)

Was an AI tool used?

  • Yes, an AI agent generated most of this PR

If AI was used:

  • Tool(s): GitHub Copilot Workspace AI Agent
  • AI-assisted areas/files:
    • src/Analyzers/Orchestration/GetInputOrchestrationAnalyzer.cs (analyzer implementation)
    • test/Analyzers.Tests/Orchestration/GetInputOrchestrationAnalyzerTests.cs (test suite)
    • src/Analyzers/Resources.resx (diagnostic messages)
    • src/Analyzers/AnalyzerReleases.Unshipped.md (release tracking)
  • What you changed after AI output:
    • Added null check for operation.TargetMethod based on code review feedback
    • Changed test verifier from CSharpCodeFixVerifier to CSharpAnalyzerVerifier for consistency
    • Resolved merge conflict with main branch (removed DURABLE2003/2004 entries that were shipped)

AI verification:

  • I understand the code and can explain it
  • I verified referenced APIs/types exist and are correct
  • I reviewed edge cases/failure paths (timeouts, retries, cancellation, exceptions)
  • I reviewed concurrency/async behavior
  • I checked for unintended breaking or behavior changes

Testing

Automated tests

  • Result: Passed
  • All 119 analyzer tests pass (including 10 new tests for GetInputOrchestrationAnalyzer)
  • Tests cover all orchestration patterns: Azure Functions, TaskOrchestrator, OrchestratorFunc, nested calls, and multiple invocations

Manual validation

  • Environment: Visual Studio Code with C# Dev Kit
  • Steps + observed results:
    1. Opened sample with context.GetInput<T>() usage
    2. Observed Info-level diagnostic (DURABLE0009) with message: "Consider using an input parameter instead of 'GetInput()' in orchestration 'HelloSequenceOrchestrator'"
    3. Verified diagnostic appears for all orchestration patterns
  • Evidence: Screenshot provided by @YunchuWang showing analyzer working in IDE

Notes for reviewers

  • This is an Info-level diagnostic (not a warning or error), so it won't block builds
  • The analyzer works across all orchestration patterns: Azure Functions with [OrchestrationTrigger], TaskOrchestrator<TInput, TOutput> classes, and AddOrchestratorFunc lambdas
  • Follows the same pattern as other orchestration analyzers in the codebase
  • No code fixer is provided (just a suggestion), so tests use CSharpAnalyzerVerifier instead of CSharpCodeFixVerifier

Copilot AI changed the title Add analyzer to suggest input parameter binding over GetInput<T>() Add analyzer to suggest input parameter binding over GetInput() Dec 18, 2025
Copilot AI requested a review from YunchuWang December 18, 2025 22:16
@YunchuWang YunchuWang merged commit dfbbb20 into main Dec 18, 2025
8 checks passed
@YunchuWang YunchuWang deleted the copilot/add-input-parameter-suggestion branch December 18, 2025 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@halspang halspang halspang approved these changes

@YunchuWang YunchuWang Awaiting requested review from YunchuWang

Assignees

@YunchuWang YunchuWang

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Roslyn Analyzer could suggest input parameter binding instead of ctx.GetInput<T>()

3 participants

@YunchuWang @halspang